fix(memory): Fix memory leak for RecorderClass::m_crcInfo#2713
Open
Caball009 wants to merge 8 commits into
Open
fix(memory): Fix memory leak for RecorderClass::m_crcInfo#2713Caball009 wants to merge 8 commits into
Caball009 wants to merge 8 commits into
Conversation
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Include/Common/Recorder.h | CRCInfo moved from .cpp file-local class to nested protected class; m_crcInfo changed from pointer to value member; default constructor added. |
| Generals/Code/GameEngine/Source/Common/Recorder.cpp | Old file-local CRCInfo class removed; CRCInfo method bodies moved to scoped RecorderClass::CRCInfo::; all m_crcInfo-> uses replaced with m_crcInfo.*; playbackFile() uses value assignment instead of NEW. |
| GeneralsMD/Code/GameEngine/Include/Common/Recorder.h | Identical structural change to the Generals header — CRCInfo nested in RecorderClass, m_crcInfo as value member. |
| GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp | Mirror of the Generals .cpp change — old file-local CRCInfo removed, method bodies re-scoped, pointer access replaced with value access. |
Class Diagram
%%{init: {'theme': 'neutral'}}%%
classDiagram
class RecorderClass {
#CRCInfo m_crcInfo
+RecorderClass()
+~RecorderClass()
+playbackFile(filename) Bool
+handleCRCMessage(newCRC, playerIndex, fromPlayback)
+sawCRCMismatch() Bool
}
class CRCInfo {
#Bool m_sawCRCMismatch
#Bool m_skippedOne
#UnsignedInt m_localPlayer
#list~UnsignedInt~ m_data
+CRCInfo()
+CRCInfo(localPlayer, isMultiplayer)
+addCRC(val)
+readCRC() UnsignedInt
+GetQueueSize() int
+getLocalPlayer() UnsignedInt
+setSawCRCMismatch()
+sawCRCMismatch() Bool
}
RecorderClass *-- CRCInfo : nested class (value member)
note for RecorderClass "BEFORE: CRCInfo* m_crcInfo (heap via NEW, never deleted)\nAFTER: CRCInfo m_crcInfo (value member, auto-managed)"
Reviews (6): Last reviewed commit: "Tweaked 'CRCInfo' TSH comment." | Re-trigger Greptile
Caball009
force-pushed
the
fix_memory_leak_crcInfo
branch
from
282478b to
c5e3079
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
xezon
reviewed
May 15, 2026
xezon
approved these changes
May 17, 2026
xezon
left a comment
xezon
left a comment
There was a problem hiding this comment.
Looking good. Needs replicate to Generals.
Caball009
force-pushed
the
fix_memory_leak_crcInfo
branch
from
e99381e to
82efc2a
Compare
This avoids an unnecessary dynamic memory allocation and fixes a memory leak.
…aks to the latter.
Author
|
Rebased to include the fix for the CI Replay checker. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a memory leak for
RecorderClass::m_crcInfo. I decided to fix this one by not allocating theCRCInfoobject dynamically. This did require a larger change of moving the class to the header, though.See commits for clean diffs.
TODO: